#25: implement tool commandlet for intellij #297

ndemirca · 2024-04-17T09:40:54Z

Closes #25

…llij

salimbouch

Looks great to me 👍

cli/src/main/java/com/devonfw/tools/ide/tool/intellij/Intellij.java

…llij

coveralls · 2024-04-23T10:01:08Z

Pull Request Test Coverage Report for Build 9764854954

Details

0 of 0 changed or added relevant lines in 0 files are covered.
22 unchanged lines in 3 files lost coverage.
Overall coverage increased (+0.7%) to 60.875%

Files with Coverage Reduction	New Missed Lines	%
com/devonfw/tools/ide/tool/intellij/Intellij.java	3	87.5%
com/devonfw/tools/ide/io/FileAccess.java	4	63.64%
com/devonfw/tools/ide/io/FileAccessImpl.java	15	55.08%

Totals
Change from base Build 9761010934:	0.7%
Covered Lines:	5064
Relevant Lines:	8008

💛 - Coveralls

salimbouch

Looks great to me 👍

…llij

added copy recursively with folder name fixed test workarounds removed extra resolve directory workarounds

…nt-ToolCommandlet-for-Intellij

…llij

hohwille

@ndemirca Thanks for your improvements and all the great work in this PR. 👍
However, we still have some last mile to go to complete this story.
See my review comments for details.

.gitattributes

hohwille · 2024-06-13T10:07:15Z

cli/src/main/java/com/devonfw/tools/ide/tool/intellij/Intellij.java

+    } else if (this.context.getSystemInfo().isLinux()) {
+      return toolBinPath.resolve(IDEA_BASH_SCRIPT).toString();
+    } else {
+      return getToolPath().resolve("IntelliJ IDEA CE.app").resolve("Contents").resolve("MacOS").resolve(IDEA).toString();


IMHO this is still wrong as we discussed in our meeting.
I already merged the PR #359 and resolved the conflicts into this PR.
If we did everything correct there is no such thing anymore than IntelliJ IDEA CA.app inside the software folder.
Also please be aware the different editions of intelliJ will have a different *.app name so your code would never work with the ultimate edition used by many projects (CE stands for Community Edition).
I hope it was already clarified meanwhile why we need the generic MacOS workaround and avoid such ugly OS-switches here and handling of MacOS apps in individual commandlets.
Ideally like in devonfw-ide after installation we can just call idea on any plattform to launch IntelliJ.
This PR therefore still needs some reworking...

I've added a very simple workaround for mac ultimate and community/intellij edition.
The dmg extractor still keeps the IntelliJ IDEA CE.app/Contents/... in the software folder, so this way should suffice for now. For another method to ideally call it with idea on each platform, we need to do some more research and discussions.

For further improvements I've added an issue: #440

hohwille · 2024-06-13T10:08:00Z

cli/src/main/java/com/devonfw/tools/ide/tool/intellij/Intellij.java

+    if (this.context.getSystemInfo().isMac()) {
+      setMacOsFilePermissions(getToolPath().resolve("IntelliJ IDEA CE.app").resolve("Contents").resolve("MacOS").resolve(IDEA));
+    }


same here...

As described above.

hohwille · 2024-06-13T10:26:57Z

cli/src/main/java/com/devonfw/tools/ide/tool/intellij/Intellij.java


+    if (Files.exists(binaryFile)) {
+      String permissionStr = FileAccessImpl.generatePermissionString(493);


Seems I missed to discuss on this API.
IMHO this is very bad design:

numeric file permissions must always be given in octal notation or nobody can understand what 493 actually is. If you write it octal as 0755 at least some bash/unix experienced user will understand what this.

We should avoid direct usage of FileAccessImpl in commandlets. That is why we have FileAccess interface.

Why do we provide a method in FileAccessImpl that converts a number to a String and then we again convert the String to a Set of PosixFilePermissions in order to then natively set this via Files.setPosixFilePermissions? The idea of FileAccess is to create a higher level API and abstraction to such lower-level Java NIO operations and make our live easier. So if we believe that using numeric (octal) notation is the most readable way why dont we create a method in FileAccess to set file permissions to a given file with a given numeric value? However, it may be even simpler and more readable to simply define constants for the combinations that we need like this:

/** {@link PosixFilePermission}s for "rwxr-xr-x" or 0755. */ public static Set<PosixFilePermission> RWX_RX_RX = Set.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE, PosixFilePermission.GROUP_READ, PosixFilePermission.GROUP_EXECUTE, PosixFilePermission.OTHERS_READ, PosixFilePermission.OTHERS_EXECUTE);

My real suggestion would rather be to add a method FileAccess.makeExecutable(Path) that will simply implement what chmod a+x «path» will do and could also be implemented to work on Windows (see also here).

I've implemented your suggestions now.

hohwille · 2024-06-13T10:27:30Z

cli/src/main/java/com/devonfw/tools/ide/tool/intellij/Intellij.java

+      } finally {
+        return;


This does not seem to add any value and is IMHO only causing confusion.

Suggested change

} finally {

return;

hohwille · 2024-06-13T10:30:03Z

cli/src/main/java/com/devonfw/tools/ide/tool/intellij/Intellij.java

+  @Override
+  public void installPlugin(PluginDescriptor plugin) {
+


This is not implemented so this PR does not entirely close #25. If you do something like this on purpose, then create a new issue for IntelliJ plugin support and leave a comment here like TODO https://github.com/devonfw/IDEasy/issues/«id». Otherwise we still end up in the same situation that we have currently that some comandlets seem to be present but are incomplete and if we have no issue about the problem, we might forget about it and currently merging this PR would automatically close #25.

Adjusted with new issue.

…llij # Conflicts: # cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java # cli/src/main/java/com/devonfw/tools/ide/tool/LocalToolCommandlet.java

…thub.com:ndemirca/IDEasy into feature/25-Implement-ToolCommandlet-for-Intellij

added new makeExecutable method to FileAccess added new RWX_RX_RX constant to FileAccess replaced complicated sert of execute permissions in Intellij with makeExecutable method added TODO for the implementation of plugin installation applid reformat to FileAccess

…llij

updated .gitattributes studio64.exe Co-authored-by: Jörg Hohwiller <hohwille@users.noreply.github.com>

ndemirca and others added 7 commits February 23, 2024 16:06

devonfw#25: implemented install and plugins

13a2b75

Merge branch 'main' into feature/25-Implement-ToolCommandlet-for-Inte…

b8292b6

…llij

added first windows unittests for intellij

bcf0e4b

tested intellij installation

1194c04

adapted test data for intellij commandlet

2213057

Adapted intellij tests

4329115

Merge branch 'main' into feature/25-Implement-ToolCommandlet-for-Inte…

b1f7e9c

…llij

ndemirca marked this pull request as draft April 17, 2024 09:41

ndemirca added 3 commits April 17, 2024 11:49

adjusted test data

a1ae18a

changed test data for mac

3402cc7

fixed intellij Unittests

2d39454

ndemirca marked this pull request as ready for review April 17, 2024 11:46

ndemirca requested a review from salimbouch April 17, 2024 11:47

ndemirca marked this pull request as draft April 17, 2024 14:38

salimbouch reviewed Apr 17, 2024

View reviewed changes

cli/src/main/java/com/devonfw/tools/ide/tool/intellij/Intellij.java Outdated Show resolved Hide resolved

cli/src/main/java/com/devonfw/tools/ide/tool/intellij/Intellij.java Outdated Show resolved Hide resolved

ndemirca added 3 commits April 17, 2024 18:25

expanden intellij tool commandlet

0bd0b42

added javadoc

1599acb

Merge branch 'main' into feature/25-Implement-ToolCommandlet-for-Inte…

2dbd602

…llij

added improvements to intellij and optimized tests

c4ae3cf

ndemirca marked this pull request as ready for review April 23, 2024 11:55

ndemirca added 2 commits April 23, 2024 14:23

cleaned up test data

e26ce93

imroved tests

7b63574

ndemirca marked this pull request as draft April 23, 2024 15:51

salimbouch approved these changes Apr 23, 2024

View reviewed changes

ndemirca added 5 commits April 24, 2024 12:13

fixed mac implementation

93265c9

added file mode to extractDmg to fix a bug

dd5b979

improved mac implementation

ab2bedd

refactoring

5f5b2a9

adapted tests

a87dde9

ndemirca added 6 commits May 7, 2024 12:30

fixed mac implementation

9f5d2f7

fixed and cleaned up tests

1664414

manipulated jmc test file

548357d

reverted test step for jmc tests

35c7d63

added node_modules into gittignore

480a472

Merge branch 'main' into feature/25-Implement-ToolCommandlet-for-Inte…

408b8e7

…llij

ndemirca marked this pull request as ready for review May 8, 2024 12:27

ndemirca requested a review from hohwille May 17, 2024 08:47

ndemirca and others added 11 commits May 17, 2024 14:19

Merge branch 'main' into feature/25-Implement-ToolCommandlet-for-Inte…

30e4048

…llij

fixed copy bug in extractdmg

20e8710

changed binarypath for mac

a45b108

fixed copy method within extractdmg

bf7fa79

adapted mac binary path

26e902f

devonfw#347: fixed copy

3382e54

added copy recursively with folder name fixed test workarounds removed extra resolve directory workarounds

Merge branch 'feature/347-Change-copy-method' into feature/25-Impleme…

40eb875

…nt-ToolCommandlet-for-Intellij

fixed copy leftover and refactoring

30062f3

reverted refactoring

ad6f549

fixed mac tests

63793c8

Merge branch 'main' into feature/25-Implement-ToolCommandlet-for-Inte…

9ca87e3

…llij

hohwille requested changes Jun 13, 2024

View reviewed changes

mvomiero assigned jan-vcapgemini Jul 1, 2024

jan-vcapgemini added 3 commits July 1, 2024 11:16

Merge branch 'main' into feature/25-Implement-ToolCommandlet-for-Inte…

920ccfa

…llij # Conflicts: # cli/src/main/java/com/devonfw/tools/ide/io/FileAccessImpl.java # cli/src/main/java/com/devonfw/tools/ide/tool/LocalToolCommandlet.java

Merge branch 'feature/25-Implement-ToolCommandlet-for-Intellij' of gi…

2714aa4

…thub.com:ndemirca/IDEasy into feature/25-Implement-ToolCommandlet-for-Intellij

hohwille added this to the release:2024.07.002 milestone Jul 2, 2024

jan-vcapgemini and others added 4 commits July 2, 2024 15:36

Merge branch 'main' into feature/25-Implement-ToolCommandlet-for-Inte…

2947503

…llij

devonfw#25: fixed copy method

d9c8268

devonfw#25: added mac edition handling

bd487c6

devonfw#25: implemented requested change

5c956eb

updated .gitattributes studio64.exe Co-authored-by: Jörg Hohwiller <hohwille@users.noreply.github.com>

jan-vcapgemini requested a review from hohwille July 2, 2024 16:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#25: implement tool commandlet for intellij #297

#25: implement tool commandlet for intellij #297

ndemirca commented Apr 17, 2024 •

edited

Loading

salimbouch left a comment

coveralls commented Apr 23, 2024 •

edited

Loading

salimbouch left a comment

hohwille left a comment

hohwille Jun 13, 2024 •

edited

Loading

jan-vcapgemini Jul 2, 2024

jan-vcapgemini Jul 2, 2024

hohwille Jun 13, 2024

jan-vcapgemini Jul 2, 2024

hohwille Jun 13, 2024

jan-vcapgemini Jul 2, 2024

hohwille Jun 13, 2024

jan-vcapgemini Jul 2, 2024

hohwille Jun 13, 2024

jan-vcapgemini Jul 2, 2024


		if (Files.exists(binaryFile)) {
		String permissionStr = FileAccessImpl.generatePermissionString(493);

		@Override
		public void installPlugin(PluginDescriptor plugin) {

#25: implement tool commandlet for intellij #297

Are you sure you want to change the base?

#25: implement tool commandlet for intellij #297

Conversation

ndemirca commented Apr 17, 2024 • edited Loading

salimbouch left a comment

Choose a reason for hiding this comment

coveralls commented Apr 23, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9764854954

Details

💛 - Coveralls

salimbouch left a comment

Choose a reason for hiding this comment

hohwille left a comment

Choose a reason for hiding this comment

hohwille Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ndemirca commented Apr 17, 2024 •

edited

Loading

coveralls commented Apr 23, 2024 •

edited

Loading

hohwille Jun 13, 2024 •

edited

Loading